[pigeon] updates toString and isNullish methods by tarrinneal · Pull Request #11625 · flutter/packages

tarrinneal · 2026-04-30T22:57:06Z

prequel pr to #11352 to land non NI changes to simplify pr.

gemini-code-assist

Code Review

This pull request updates Pigeon to version 26.4.0, introducing automatic generation of toString (or equivalent) methods for data classes across all supported languages. It also refines the Swift isNullish utility to handle nested optional values. Review feedback highlights a critical logic error in the C++ generator that results in unbalanced braces and corrupted code, along with a compilation issue when handling C++ enums. Additionally, for Swift, the feedback identifies a redundant protocol conformance error in subclasses and suggests using String(describing:) to prevent compiler warnings during string interpolation of optional fields.

gemini-code-assist · 2026-04-30T22:58:54Z

+                indent.writeln('ss << *$name;');
+              }
+            });
+            indent.nest(1, () {
+              indent.writeln('ss << "null";');
+            });
+          } else {
+            if (field.type.isClass) {
+              indent.writeln('ss << $name.ToString();');
+            } else {
+              indent.writeln('ss << $name;');


C++ enum class types do not support the << operator for std::ostream by default. If a data class contains an enum field, the generated ToString method will fail to compile. You should check if the field is an enum and cast it to its underlying integer type before appending it to the stream.

Gemini is correct here. You'll also need custom handling of maps, and maybe lists.

gemini-code-assist · 2026-04-30T22:58:54Z

+      final Iterable<String> fieldStrings = classDefinition.fields.map((
+        NamedType field,
+      ) {
+        return '${field.name}: \\(${field.name})';


In Swift, string interpolation of optional values triggers a compiler warning (e.g., "Expression implicitly coerced from 'String?' to 'Any'"). It is recommended to wrap optional fields in String(describing:) to silence this warning and provide a consistent string representation.

Suggested change

return '${field.name}: \\(${field.name})';

return '${field.name}: \(String(describing: ${field.name}))';

stuartmorgan-g

The native unit tests should have a test that calls the to-string method for a test object and ensure that the expected string results, so that we are validating that the methods do what we expect (and also have a clear snapshot of what the expected output actually is).

stuartmorgan-g · 2026-05-04T15:11:55Z

+                indent.writeln('ss << *$name;');
+              }
+            });
+            indent.nest(1, () {
+              indent.writeln('ss << "null";');
+            });
+          } else {
+            if (field.type.isClass) {
+              indent.writeln('ss << $name.ToString();');
+            } else {
+              indent.writeln('ss << $name;');


Gemini is correct here. You'll also need custom handling of maps, and maybe lists.

stuartmorgan-g · 2026-05-04T15:21:15Z

+        NamedType field,
+      ) {
+        if (_usesPrimitive(field.type)) {
+          return '@(self.${field.name})';


FWIW it would be conceptually cleaner to use the right format string for each primitive type, rather than boxing them just to print them as objects. But since this is just a nice-to-have utility method, and Obj-C isn't a development focus, it's fine to leave as-is.

lmk if my change is what you had in mind.

tarrinneal · 2026-05-13T02:47:19Z

/gemini review

gemini-code-assist

Code Review

This pull request implements toString (or equivalent) method generation for data classes across all supported Pigeon generators, including Dart, Java, Kotlin, Swift, Objective-C, C++, and GObject. It also updates the Swift isNullish helper to correctly handle double-nested Any? values and adds new list-based echo methods to the core integration tests. Feedback identifies a missing nullability check for boolean fields in the GObject generator and potential integer truncation issues in the Objective-C description implementation on 32-bit platforms. Additionally, a suggestion was made to maintain field name consistency in GObject string representations.

gemini-code-assist · 2026-05-13T02:54:02Z

+        } else if (field.type.baseName == 'bool') {
+          indent.writeln(
+            'g_string_append(str, self->$fieldName ? "true" : "false");',
+          );


The bool case is missing nullability handling. In the GObject generator, nullable booleans are represented as gboolean*. Using self->fieldName directly in a ternary will check the pointer's validity rather than the boolean value, and it will print "false" instead of "null" when the pointer is null.

} else if (field.type.baseName == 'bool') { if (field.type.isNullable) { indent.writeScoped('if (self->$fieldName != nullptr) {', '}', () { indent.writeln( 'g_string_append(str, *self->$fieldName ? "true" : "false");', ); }); indent.writeScoped('else {', '}', () { indent.writeln('g_string_append(str, "null");'); }); } else { indent.writeln( 'g_string_append(str, self->$fieldName ? "true" : "false");', ); } }

gemini-code-assist · 2026-05-13T02:54:02Z

+            case 'bool':
+              return '${field.name}: %@';
+            case 'int':
+              return '${field.name}: %ld';


Pigeon int fields are generated as int64_t in Objective-C. Using %ld with a (long) cast can lead to integer truncation on 32-bit platforms. It is safer to use %lld with a (long long) cast to ensure full 64-bit precision is maintained in the description.

Suggested change

return '${field.name}: %ld';

case 'int':

return '${field.name}: %lld';

gemini-code-assist · 2026-05-13T02:54:02Z

+            case 'bool':
+              return 'self.${field.name} ? @"true" : @"false"';
+            case 'int':
+              return '(long)self.${field.name}';


As noted above, int64_t should be cast to (long long) when used with the %lld format specifier to avoid truncation on 32-bit architectures.

Suggested change

return '(long)self.${field.name}';

case 'int':

return '(long long)self.${field.name}';

gemini-code-assist · 2026-05-13T02:54:02Z

+      enumerate(classDefinition.fields, (int index, final NamedType field) {
+        final String fieldName = _getFieldName(field.name);
+        final comma = index == 0 ? '' : ', ';
+        indent.writeln('g_string_append(str, "$comma$fieldName: ");');


For consistency with other generated platforms (Dart, Java, Kotlin, Swift, C++), the label in the string representation should use the original field name (field.name) rather than the snake_case GObject member name (fieldName).

Suggested change

indent.writeln('g_string_append(str, "$comma$fieldName: ");');

indent.writeln('g_string_append(str, "$comma${field.name}: ");');

toString and isNullish

b78d7e9

tarrinneal requested a review from stuartmorgan-g April 30, 2026 22:57

flutter-dashboard Bot added the CICD Run CI/CD label Apr 30, 2026

github-actions Bot added p: pigeon platform-android platform-ios platform-linux platform-macos platform-windows labels Apr 30, 2026

gemini-code-assist Bot reviewed Apr 30, 2026

View reviewed changes

tarrinneal mentioned this pull request Apr 30, 2026

Add FFI and JNI support to Swift and Kotlin #11352

Open

gemini has good comments this time

ebc37cc

github-actions Bot removed the CICD Run CI/CD label May 1, 2026

stuartmorgan-g added the CICD Run CI/CD label May 4, 2026

stuartmorgan-g requested changes May 4, 2026

View reviewed changes

add missing tests for lists

7eab5eb

github-actions Bot removed the CICD Run CI/CD label May 7, 2026

extend c++ and gobject methods to deeper depths

ae8ea57

flutter-dashboard Bot added the CICD Run CI/CD label May 11, 2026

objc primitive type specifics

9b18f15

github-actions Bot removed the CICD Run CI/CD label May 12, 2026

all feedback.

147ec0f

flutter-dashboard Bot added the CICD Run CI/CD label May 13, 2026

tarrinneal requested a review from stuartmorgan-g May 13, 2026 02:48

gemini-code-assist Bot reviewed May 13, 2026

View reviewed changes

	return '${field.name}: \\(${field.name})';
	return '${field.name}: \(String(describing: ${field.name}))';

	return '${field.name}: %ld';
	case 'int':
	return '${field.name}: %lld';

	return '(long)self.${field.name}';
	case 'int':
	return '(long long)self.${field.name}';

	indent.writeln('g_string_append(str, "$comma$fieldName: ");');
	indent.writeln('g_string_append(str, "$comma${field.name}: ");');

Conversation

tarrinneal commented Apr 30, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gemini-code-assist Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stuartmorgan-g May 4, 2026

Choose a reason for hiding this comment

Uh oh!

tarrinneal May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tarrinneal commented May 13, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants